Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix small remarks #3

Merged
merged 14 commits into from
Jun 6, 2024
Merged

Fix small remarks #3

merged 14 commits into from
Jun 6, 2024

Conversation

gaetbout
Copy link
Contributor

@gaetbout gaetbout commented Jun 6, 2024

Addressing some remarks

@gaetbout gaetbout requested review from sgc-code and Leonard-Pat June 6, 2024 07:14
Comment on lines +99 to +101

fn compute_max_fee_v3(mut resource_bounds: Span<ResourceBounds>, tip: u128) -> u128 {
let mut max_fee: u128 = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why move from the utils ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it can be part of the account now that we reduced its code and doesn't need to be shared anymore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree it belongs here

assert(max_fee.into() < amount, 'gift-fac/fee-too-high');

let sender = get_caller_address();
let factory = get_contract_address();
// TODO We could manually serialize for better performance
// TODO pubkey can be zero?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is solved ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref

@gaetbout
Copy link
Contributor Author

gaetbout commented Jun 6, 2024

Don't merge please

@gaetbout
Copy link
Contributor Author

gaetbout commented Jun 6, 2024

Fixing tests

@gaetbout
Copy link
Contributor Author

gaetbout commented Jun 6, 2024

Done (but one that should be fixed with Leo's PR)

@sgc-code sgc-code merged commit c037542 into develop Jun 6, 2024
3 of 5 checks passed
@sgc-code sgc-code deleted the pr-quick-fixes branch July 5, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants